Bdms 227 transfer updates#257
Conversation
…zations for contacts
parse the construction notes field to get pump types. this can be more sophisticated, but that's a future problem. for now just check for some basic pump type string patterns
This can be updated later on but should be "Air-Rotary" for the transfer
this was an artifact from merge conflicts
Laila indicated that this just describes if a datalogger can be physically installed at the well. It does not pertain to permissions
| # from schemas.geologic_formation import ( | ||
| # GeologicFormationResponse, | ||
| # ) |
There was a problem hiding this comment.
Don't we still need a GeologicFormationResponse?
There was a problem hiding this comment.
I don't think we do. The feature file states that it should just return the formation codes, so I parse all of the geologic formation and just return the codes as plain strings
| GeologicFormationResponse, | ||
| ThingGeologicFormationAssociationResponse, | ||
| ) | ||
| from schemas.aquifer_system import AquiferSystemGeoJSONResponse |
There was a problem hiding this comment.
I thought we decided to forego the GeoJSON response?
There was a problem hiding this comment.
This was used in the WellResponse and I forgot it was there. I've now changed it so that a string of the aquifer system name is returned
| allow_water_level_samples: bool | None | ||
| allow_water_chemistry_samples: bool | None | ||
| allow_datalogger_installation: bool | None | ||
| is_suitable_for_datalogger: bool | None |
There was a problem hiding this comment.
These look like fields from the previous PermissionHistory table design. Should these fields be removed and replaced with permission_type and/or permission_allowed?
There was a problem hiding this comment.
Sorry if you are getting this twice. I somehow started a review of my own PR, which I just deleted so now I need to comment again:
The steps in the feature file are
And the response should include whether repeat measurement permission is granted for the well
And the response should include whether sampling permission is granted for the well
And the response should include whether datalogger installation permission is granted for the well
so I'm just returning boolean values based off of the latest response where end_date is None. This data is, however, stored in the PermisionHistory table.
@chasetmartin do you want the response to be single fields like this, or would you prefer that it more closely reflect the database design? If so I can change it to
{
...
"permissions": [
{"permission_type": "Water Level Sample", "permission_allowed": True/False},
{"permission_type": "Water Chemistry Sample", "permission_allowed": True/False},
{"permission_type": "Datalogger Installation", "permission_allowed": True/False}
]
}
There was a problem hiding this comment.
@jacob-a-brown I like the later, the list of permission objects, I think it's almost more intuitive to read/parse. What do you think @TylerAdamMartinez ?
There was a problem hiding this comment.
Same here, let's make it more closely resemble the database design.
There was a problem hiding this comment.
Do you want it to return all permissions, even if they have ended? Or just the latest permissions (that is, latest start_date where end_date is None)?
There was a problem hiding this comment.
the permissions are now returned in a list
{
...
"permissions": [
{"permission_type": "Water Level Sample", "permission_allowed": True/False, "start_date": date, "end_date": None},
{"permission_type": "Water Chemistry Sample", "permission_allowed": True/False, "start_date": date, "end_date": None},
{"permission_type": "Datalogger Installation", "permission_allowed": True/False, "start_date": date, "end_date": None}
]
}- Only the latest record (based off of
start_date) whereend_dateisNoneis returned for each type - If the permission record does not exist for a particular
permission_typethenpermission_allowed,start_date, andend_dateareNone. - while
start_dateis non-nullable in the database, it must be nullable in the response incase there are no permissions of particular type end_datewill always beNonefor active permissions. I'm including it in thePermissionHistoryresponse, though, so that if we want to list out the full permission history for an object it'll be there
ksmuczynski
left a comment
ksmuczynski
left a comment
There was a problem hiding this comment.
Overall looks good! Left a few comments.
the well screen response includes the aquifer and geology information, but just as strings of names and codes instead of the full nested response objects.
4 participants
all transfers for additional well information except for aquifer/geology data. this depends on PR #256 being merged since it incorporates api/model/schema changes made for the tests to pass